Skip to content

Conversation

@nomis
Copy link

@nomis nomis commented Apr 19, 2016

When a single webserver is used to access multiple FPM pools (one per user), it can become possible for one vhost to execute scripts using the pool of another vhost (e.g. Apache SetHandler in .htaccess).

To prevent this, it is necessary to restrict which files a pool can execute for a request (it doesn't matter what that file then does as long as it is under control of the correct user).

While chroot does achieve this it then becomes inconvenient to allow access to any external resources (e.g. database sockets, libraries, applications) without inadvertently allowing third party users to make files available within the chroot.

This adds a per-pool configuration option "security.exec_basedir":
Limits the directory which can be used to execute the request script.
This can be used to ensure that pools defined for specific users can
only be used to execute scripts under the control of those users.
This value must be defined as an absolute path and end with a '/'.

@nomis nomis force-pushed the fpm/security.exec_basedir branch from 2590cb5 to 3492786 Compare April 23, 2016 11:49
When a single webserver is used to access multiple FPM pools (one per
user), it can become possible for one vhost to execute scripts using
the pool of another vhost (e.g. Apache SetHandler in .htaccess).

To prevent this, it is necessary to restrict which files a pool can
execute for a request (it doesn't matter what that file then does as
long as it is under control of the correct user).

While chroot does achieve this it then becomes inconvenient to allow
access to any external resources (e.g. database sockets, libraries,
applications) without inadvertently allowing third party users to make
files available within the chroot.

This adds a per-pool configuration option "security.exec_basedir":
  Limits the directory which can be used to execute the request script.
  This can be used to ensure that pools defined for specific users can
  only be used to execute scripts under the control of those users.
  This value must be defined as an absolute path and end with a '/'.
@nomis nomis force-pushed the fpm/security.exec_basedir branch from 3492786 to faa938d Compare May 11, 2016 18:54
@jpauli jpauli added the Feature label Jul 11, 2016
return 0; /* allowed by default */
}

if (strncmp(path, exec_basedir, strlen(exec_basedir)) == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible read-overflow if strlen(path) < strlen(exec_basedir)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how strncmp works. It will never read off the end of either string.

@remicollet
Copy link
Member

This probably worth to be discussed as part of the RFC process.
https://wiki.php.net/rfc/howto

@remicollet
Copy link
Member

If you have 2 pools, with 2 users, unix permissions on script should be enough to protect user1 from reading user2 files.

@nomis
Copy link
Author

nomis commented Jul 11, 2016

Permissions on which script?

If you have two pools both being accessed by the same Apache instance then either user can use a .htaccess file to access the fpm proxy using SetHandler "proxy:unix:/path/to/fpm..." and execute their own scripts using the other user's pool.

@nomis
Copy link
Author

nomis commented Jul 11, 2016

I don't intend to submit an RFC as this is not a change to the PHP language.

@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

Since the author is unwilling to engage in the proper process to have this change merged, I'm closing this PR.

@krakjoe krakjoe closed this Jan 3, 2017
@nomis
Copy link
Author

nomis commented Jan 3, 2017

A single instance of Apache can only run as one user/group so the same listen.group value has to be used for all pools. There is no built-in protection within Apache to stop vhosts from using arbitrary "proxy:unix:*" handlers.

This a major security issue because there is no way to restrict which pool each vhost is allowed to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants